Skip to content

fix(wall): bound CPED freelist with dynamic budget + hysteresis#258

Merged
szegedi merged 5 commits intomainfrom
BridgeAR/2026-02-03-fix-memory-leak
Feb 5, 2026
Merged

fix(wall): bound CPED freelist with dynamic budget + hysteresis#258
szegedi merged 5 commits intomainfrom
BridgeAR/2026-02-03-fix-memory-leak

Conversation

@BridgeAR
Copy link

@BridgeAR BridgeAR commented Feb 3, 2026

@BridgeAR BridgeAR requested a review from szegedi as a code owner February 3, 2026 16:50
@BridgeAR BridgeAR added the semver-patch Bug or security fixes, mainly label Feb 3, 2026
@github-actions
Copy link

github-actions bot commented Feb 3, 2026

Overall package size

Self size: 1.79 MB
Deduped: 2.17 MB
No deduping: 2.17 MB

Dependency sizes | name | version | self size | total size | |------|---------|-----------|------------| | source-map | 0.7.6 | 185.63 kB | 185.63 kB | | pprof-format | 2.2.1 | 163.06 kB | 163.06 kB | | p-limit | 3.1.0 | 7.75 kB | 13.78 kB | | delay | 5.0.0 | 11.17 kB | 11.17 kB | | node-gyp-build | 3.9.0 | 8.81 kB | 8.81 kB |

🤖 This report was automatically generated by heaviest-objects-in-the-universe

@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026

Benchmarks

Benchmark execution time: 2026-02-03 17:06:10

Comparing candidate commit e3edd0c in PR branch BridgeAR/2026-02-03-fix-memory-leak with baseline commit 947f5cf in branch main.

Found 0 performance improvements and 1 performance regressions! Performance is the same for 89 metrics, 30 unstable metrics.

scenario:profiler-light-load-with-wall-profiler-22

  • 🟥 cpu_user_time [+2.672ms; +10.648ms] or [+2.488%; +9.916%]

@BridgeAR BridgeAR marked this pull request as draft February 3, 2026 17:16
@BridgeAR BridgeAR marked this pull request as ready for review February 3, 2026 19:15
@RDIL
Copy link

RDIL commented Feb 5, 2026

👋 @BridgeAR thanks for making this PR - invested onlooker from the linked issue here. Trying to figure out if this will fix things on our app. Could you provide a bit more detail about what the problem was and how it was fixed? Was this only Node 24? Was there a specific commit that caused this to start happening? etc. Thank you!

@szegedi
Copy link

szegedi commented Feb 5, 2026

hey @RDIL, the relevant dd-trace-js commit that enabled this behavior is DataDog/dd-trace-js#7119. It's on by default on Node.js 24+. (It will also be turned on on Node.js 22-23 but only when running Node.js with explicit --experimental-async-context-frame setting.)

If you need a workaround urgently, you can revert to older behavior on Node.js 24+ by setting DD_PROFILING_ASYNC_CONTEXT_FRAME_ENABLED=0 explicitly.

@szegedi
Copy link

szegedi commented Feb 5, 2026

#255 will make this obsolete, as the rework I did there made the freelist not feasible anymore. (FWIW, malloc itself manages a freelist, so I'm not even sure if this measure effectively fought memory fragmentation at all.)

In that PR, I started using Node.js standard ObjectWraps to hold pointers to native objects and they're deleted when their wrapper is GCd in JavaScript. We will also create way fewer native objects, as we need not install a new proxy object anymore every time ACF in CPED changes, but just when we actually change the value of the sample context in an ACF, which in [upcoming counterpart dd-trace-js changes](https://github.com/DataDog/dd-trace-js/tree/szegedi/acf-simple branch) will be just once per span, when it is activated.

Anyhow, we can integrate this PR, I'll just have to add its revert as the first commit to #255.

@BridgeAR
Copy link
Author

BridgeAR commented Feb 5, 2026

@szegedi yes, I saw #255, it just seemed like a bigger change that might need more time to actually land and I believe we need something right away.
Would you like me to take a look at 255 (I could do that at the beginning of next week)?

@szegedi szegedi merged commit eb9eabd into main Feb 5, 2026
66 of 67 checks passed
@szegedi szegedi deleted the BridgeAR/2026-02-03-fix-memory-leak branch February 5, 2026 14:29
szegedi pushed a commit that referenced this pull request Feb 5, 2026
* fix(wall): bound CPED freelist with dynamic budget + hysteresis

This implement a lower and upper bound to match different user
load situations.

Refs: DataDog/dd-trace-js#7355

* fix(wall): add bounded batch trimming + track dead counts per GC

* fix(wall): adapt trim batch with growth/decay + emergency override

* test: add memory leak test

This verifies that it fails before and is fixed with the change.

* fixup!
@szegedi szegedi mentioned this pull request Feb 5, 2026
szegedi pushed a commit that referenced this pull request Feb 5, 2026
* fix(wall): bound CPED freelist with dynamic budget + hysteresis

This implement a lower and upper bound to match different user
load situations.

Refs: DataDog/dd-trace-js#7355

* fix(wall): add bounded batch trimming + track dead counts per GC

* fix(wall): adapt trim batch with growth/decay + emergency override

* test: add memory leak test

This verifies that it fails before and is fixed with the change.

* fixup!
szegedi added a commit that referenced this pull request Feb 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver-patch Bug or security fixes, mainly

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants